GH-62: Optimize Flight SQL JDBC Statement execution paths#1090
GH-62: Optimize Flight SQL JDBC Statement execution paths#1090xborder wants to merge 9 commits intoapache:mainfrom
Conversation
* reduced number of requests for Statement * move meta orchestration * detect prepared statement
xborder
requested review from
jbonofre,
laurentgo,
lidavidm and
wgtmac
as code owners
|
Thank you for opening a pull request! Please label the PR with one or more of:
Also, add the 'breaking-change' label if appropriate. See CONTRIBUTING.md for details. |
This reverts commit b57550f.
| .withQuery(query) | ||
| .withExistingStatement(this) | ||
| .build() | ||
| .prepareAndExecute(callback); |
There was a problem hiding this comment.
Perhaps a nitpick, but if we run multiple queries with the same statement using execute we will only close one of the handles in the server.
Code for repro:
try (Connection connection = DriverManager.getConnection(jdbcUrl, properties);
Statement statement = connection.createStatement()) {
for (int i = 1; i <= n; i++) {
String sql = "SELECT " + i;
log("Executing: " + sql);
boolean isResultSet = statement.execute(sql);
if (isResultSet) {
try (ResultSet rs = statement.getResultSet()) {
// consume result set
}
} else {
System.out.println("Updated rows: " + statement.getUpdateCount());
}
}
}
A new prepared statement will be created on each query, but only one ClosePreparedStatement is sent to the server. I believe most Flight SQL servers are stateless and this won't be problematic in that scenario, but in case the server is holding some resources associated with the handle(s) of the prepared statement(s) that aren't closed there will be a leak.
Could we send the close after execution?
3 participants
What Changed
Statement.executeQuery/executeUpdateavoiding thePreparedStatementrequest flowStatement.execute(String)still flows through PreparedStatementCommandStatementUpdateorCommandStatemenQuerywithout doing a server callArrowFlightPreparedStatementArrowFlightStatementwith specific logic to handleStatementsArrowFlightMetaImpl.ArrowFlightMetaImplwas left it focused on orchestrationCloses #62.
This PR was assisted by AI